chore: add new icons to abac rooms #37424
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughRemoved the ABACHeaderTag component and its tests and i18n keys, added ABAC-aware icon selection to room icon logic (hook and private room type), updated an icons dependency, and removed ABACHeaderTag usage from room header views. Changes
Sequence Diagram(s)sequenceDiagram
participant RoomComp as Room Component
participant Hook as useRoomIcon
participant IconLogic as Icon Selection
RoomComp->>Hook: useRoomIcon(room)
Hook->>IconLogic: evaluate room.abacAttributes?
rect rgb(230,245,230)
alt abacAttributes present
IconLogic->>IconLogic: check room.teamMain
alt teamMain true
IconLogic-->>Hook: "team-shield"
else teamMain false
IconLogic-->>Hook: "hash-shield"
end
else no abacAttributes
Note right of IconLogic: proceed with existing checks (federated, prid, team, default)
IconLogic-->>Hook: other icon id
end
end
Hook-->>RoomComp: icon identifier
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37424 +/- ##
===========================================
+ Coverage 68.99% 69.03% +0.04%
===========================================
Files 3357 3356 -1
Lines 114243 114227 -16
Branches 20531 20571 +40
===========================================
+ Hits 78820 78855 +35
+ Misses 33339 33287 -52
- Partials 2084 2085 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
MartinSchoeler
force-pushed
the
chore/abac-icons
branch
from
4846d70 to
67a7757
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/meteor/client/lib/rooms/roomTypes/private.ts (1)
85-91: Consider consolidating duplicate ABAC icon logic.The ABAC icon selection logic (lines 86-91) duplicates the same logic in
apps/meteor/client/hooks/useRoomIcon.tsx(lines 12-17). Consider extracting this into a shared utility function to maintain DRY principles and ensure consistent behavior.Example refactor:
// In a shared utility file (e.g., utils/abacIcons.ts) export function getAbacIcon(room: { abacAttributes?: unknown; teamMain?: boolean }): string | null { if (!room.abacAttributes) { return null; } return room.teamMain ? 'team-shield' : 'hash-shield'; }Then use it in both locations:
const abacIcon = getAbacIcon(room); if (abacIcon) { return { name: abacIcon }; }apps/meteor/client/hooks/useRoomIcon.spec.tsx (1)
1-83: Good test coverage for ABAC icon selection.The test suite effectively validates icon selection across various room types including ABAC-enabled rooms. The structure using mock rooms and expected results makes it easy to verify behavior.
Consider adding edge case tests for:
- ABAC attributes with falsy values (empty object, null)
- Direct message rooms with a single user (testing ReactiveUserStatus component return)
- Private rooms (type 'p') with ABAC attributes
Example addition:
abacPrivateRoom: createFakeRoom({ t: 'p', name: 'abac-private', teamMain: false, // @ts-expect-error TODO: Implement ABAC attributes in rooms abacAttributes: true, }),With expected result:
abacPrivateRoom: { name: 'hash-shield' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
apps/meteor/client/components/ABAC/__snapshots__/ABACHeaderTag.spec.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
apps/meteor/client/components/ABAC/ABACHeaderTag.spec.tsx(0 hunks)apps/meteor/client/components/ABAC/ABACHeaderTag.tsx(0 hunks)apps/meteor/client/hooks/useRoomIcon.spec.tsx(1 hunks)apps/meteor/client/hooks/useRoomIcon.tsx(1 hunks)apps/meteor/client/lib/rooms/roomTypes/private.ts(1 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx(2 hunks)apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx(2 hunks)apps/meteor/client/views/room/Header/RoomHeader.tsx(0 hunks)apps/meteor/client/views/room/HeaderV2/RoomHeader.tsx(0 hunks)apps/meteor/package.json(2 hunks)packages/i18n/src/locales/en.i18n.json(0 hunks)packages/i18n/src/locales/nb.i18n.json(0 hunks)
💤 Files with no reviewable changes (6)
- apps/meteor/client/views/room/Header/RoomHeader.tsx
- apps/meteor/client/views/room/HeaderV2/RoomHeader.tsx
- apps/meteor/client/components/ABAC/ABACHeaderTag.spec.tsx
- packages/i18n/src/locales/en.i18n.json
- packages/i18n/src/locales/nb.i18n.json
- apps/meteor/client/components/ABAC/ABACHeaderTag.tsx
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/package.jsonapps/meteor/client/lib/rooms/roomTypes/private.tsapps/meteor/client/hooks/useRoomIcon.tsx
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/package.json
📚 Learning: 2025-10-24T17:32:05.348Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37299
File: apps/meteor/ee/server/lib/ldap/Manager.ts:438-454
Timestamp: 2025-10-24T17:32:05.348Z
Learning: In Rocket.Chat, ABAC attributes can only be set on private rooms and teams (type 'p'), not on public rooms (type 'c'). Therefore, when checking for ABAC-protected rooms/teams during LDAP sync or similar operations, it's sufficient to query only private rooms using methods like `findPrivateRoomsByIdsWithAbacAttributes`.
Applied to files:
apps/meteor/client/lib/rooms/roomTypes/private.tsapps/meteor/client/hooks/useRoomIcon.tsx
📚 Learning: 2025-10-27T14:38:46.994Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
Applied to files:
apps/meteor/client/lib/rooms/roomTypes/private.tsapps/meteor/client/hooks/useRoomIcon.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/client/lib/rooms/roomTypes/private.tsapps/meteor/client/hooks/useRoomIcon.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/client/hooks/useRoomIcon.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/client/hooks/useRoomIcon.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
apps/meteor/client/hooks/useRoomIcon.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/client/hooks/useRoomIcon.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/client/hooks/useRoomIcon.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/hooks/useRoomIcon.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/client/hooks/useRoomIcon.spec.tsx
🧬 Code graph analysis (1)
apps/meteor/client/hooks/useRoomIcon.spec.tsx (2)
apps/meteor/tests/mocks/data.ts (1)
createFakeRoom(46-61)apps/meteor/client/hooks/useRoomIcon.tsx (1)
useRoomIcon(8-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 [legacy watchers] (5/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 [legacy watchers] (2/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 [legacy watchers] (4/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (4/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (5/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (2/5)
🔇 Additional comments (7)
apps/meteor/client/hooks/useRoomIcon.tsx (1)
12-17: ABAC icon logic implemented correctly.The ABAC-aware icon selection properly distinguishes between team rooms ('team-shield') and regular channels ('hash-shield') when ABAC attributes are present. The early return ensures ABAC rooms display the correct icon before other room type checks.
apps/meteor/package.json (1)
257-257: No evidence found for the stated icon references in the codebase.The package versions
@rocket.chat/icons@~0.45.0and@rocket.chat/fuselage@~0.68.0both exist in the npm registry. However, the new ABAC icons 'team-shield' and 'hash-shield' are not referenced anywhere in the codebase—no TypeScript, JavaScript, JSON files, or configuration files contain these icon names. While ABAC features are actively being developed (with dedicated admin components, i18n translations, and room member logic), the specific icons mentioned are not yet integrated.Likely an incorrect or invalid review comment.
apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx (2)
4-4: Ref import mirrors AutoCompleteMonitors pattern.This change is consistent with the similar modification in
AutoCompleteMonitors.tsx. If the ref forwarding approach is validated as correct, the consistency across these components is good.However, the same concerns about type assertions apply here. Please verify that this pattern is the correct approach for enabling ref forwarding to CheckOption components.
56-69: Unable to verify external library behavior without source access.The
OptionandCheckOptioncomponents are imported from@rocket.chat/fuselage(external dependency), which is not available in the codebase. While the code pattern suggests the type assertion may be intentional—only applied where seemingly needed—I cannot definitively verify whether:
- Option's type definition already includes the correct ref type (making assertion unnecessary)
- CheckOption requires explicit casting due to different type expectations
- Both should have assertions for consistency
The concern warrants manual verification against the
@rocket.chat/fuselagecomponent documentation or source code to confirm the ref forwarding is correctly handled.apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (3)
1-46: Verify scope inconsistency: omnichannel component changes unrelated to stated ABAC icon PR objectives.The ref forwarding changes to
AutoCompleteMonitors.tsxappear to be legitimate refactoring—they mirror the existing pattern inAutoCompleteDepartmentMultiple.tsx(which already usesref={props.ref as Ref<HTMLOptionElement>}). However, these changes remain in the omnichannel scope and have no connection to the ABAC functionality described in the PR objectives (room icons, user attributes, etc.).Clarify whether:
- These omnichannel component updates should be in this PR, or
- They were included by mistake and should be removed
34-41: Type assertion pattern is consistent across all CheckOption usages, but root cause verification requires manual inspection of fuselage library types.The type assertion
as Ref<HTMLOptionElement>appears in exactly 2 locations within the codebase—both withCheckOptionin omnichannel components (AutoCompleteMonitors.tsx and AutoCompleteDepartmentMultiple.tsx). This is a 100% consistent pattern: every CheckOption usage employs the same assertion, suggesting it's intentional rather than an oversight.However, without access to fuselage's (v~0.68.0) type definitions or library documentation, I cannot definitively determine whether:
- The assertion is a known, necessary workaround for a limitation in CheckOption's ref typing
- It's correctly addressing a type incompatibility between
PaginatedMultiSelectFiltered's renderItem prop and CheckOption's ref requirements- It's masking an underlying type issue that should be resolved
To address the original concern properly, you should verify the fuselage library's CheckOption and PaginatedMultiSelectFiltered type definitions to confirm whether the assertion represents a legitimate workaround or if a deeper fix (such as wrapping CheckOption with forwardRef or updating parameter types) would be more appropriate.
3-3: Unable to verify necessity ofRefimport without access to@rocket.chat/fuselagepackage definitions.The
Reftype is used exclusively for a type assertion on line 37 (props.ref as Ref<HTMLOptionElement>). However, whether this assertion is necessary depends on howCheckOptionfrom@rocket.chat/fuselage(~0.66.4) types itsrefprop in therenderItemcallback, which cannot be verified in this environment.
MartinSchoeler
force-pushed
the
chore/abac-icons
branch
from
b0173ad to
662cc95
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/meteor/client/hooks/useRoomIcon.tsx (1)
11-11: Complete the TODO: Add abacAttributes to IRoom type definition.This issue has already been flagged in a previous review. The
@ts-expect-errordirective should be removed onceabacAttributesis properly defined in theIRoomtype from@rocket.chat/core-typings.apps/meteor/client/lib/rooms/roomTypes/private.ts (1)
85-85: Complete the TODO: Add abacAttributes to room type.This issue has already been flagged in a previous review. The
@ts-expect-errordirective should be removed onceabacAttributesis properly defined in theIRoomtype from@rocket.chat/core-typings.
🧹 Nitpick comments (1)
apps/meteor/client/lib/rooms/roomTypes/private.ts (1)
85-91: Consider extracting ABAC icon logic to a shared utility.The ABAC icon selection logic here is identical to the logic in
apps/meteor/client/hooks/useRoomIcon.tsx(lines 11-17). While both serve valid purposes in different contexts, extracting this logic to a shared utility function would reduce duplication and improve maintainability.Example shared utility in a new file (e.g.,
apps/meteor/client/lib/rooms/getABACIcon.ts):import type { IRoom } from '@rocket.chat/core-typings'; export const getABACIcon = (room: Pick<IRoom, 'abacAttributes' | 'teamMain'>): string | null => { // @ts-expect-error TODO: Implement ABAC attributes in rooms if (!room.abacAttributes) { return null; } return room.teamMain ? 'team-shield' : 'hash-shield'; };Then both
useRoomIconandPrivateRoomType.getIconcould call this utility:const abacIcon = getABACIcon(room); if (abacIcon) { return { name: abacIcon }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
apps/meteor/client/components/ABAC/__snapshots__/ABACHeaderTag.spec.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
apps/meteor/client/components/ABAC/ABACHeaderTag.spec.tsx(0 hunks)apps/meteor/client/components/ABAC/ABACHeaderTag.tsx(0 hunks)apps/meteor/client/hooks/useRoomIcon.spec.tsx(1 hunks)apps/meteor/client/hooks/useRoomIcon.tsx(1 hunks)apps/meteor/client/lib/rooms/roomTypes/private.ts(1 hunks)apps/meteor/client/views/room/Header/RoomHeader.tsx(0 hunks)apps/meteor/client/views/room/HeaderV2/RoomHeader.tsx(0 hunks)apps/meteor/package.json(1 hunks)packages/i18n/src/locales/en.i18n.json(0 hunks)packages/i18n/src/locales/nb.i18n.json(0 hunks)
💤 Files with no reviewable changes (6)
- apps/meteor/client/views/room/Header/RoomHeader.tsx
- apps/meteor/client/views/room/HeaderV2/RoomHeader.tsx
- packages/i18n/src/locales/nb.i18n.json
- packages/i18n/src/locales/en.i18n.json
- apps/meteor/client/components/ABAC/ABACHeaderTag.spec.tsx
- apps/meteor/client/components/ABAC/ABACHeaderTag.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/client/hooks/useRoomIcon.spec.tsx
- apps/meteor/package.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37299
File: apps/meteor/ee/server/lib/ldap/Manager.ts:438-454
Timestamp: 2025-10-24T17:32:05.348Z
Learning: In Rocket.Chat, ABAC attributes can only be set on private rooms and teams (type 'p'), not on public rooms (type 'c'). Therefore, when checking for ABAC-protected rooms/teams during LDAP sync or similar operations, it's sufficient to query only private rooms using methods like `findPrivateRoomsByIdsWithAbacAttributes`.
📚 Learning: 2025-10-27T14:38:46.994Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.
Applied to files:
apps/meteor/client/hooks/useRoomIcon.tsxapps/meteor/client/lib/rooms/roomTypes/private.ts
📚 Learning: 2025-10-24T17:32:05.348Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37299
File: apps/meteor/ee/server/lib/ldap/Manager.ts:438-454
Timestamp: 2025-10-24T17:32:05.348Z
Learning: In Rocket.Chat, ABAC attributes can only be set on private rooms and teams (type 'p'), not on public rooms (type 'c'). Therefore, when checking for ABAC-protected rooms/teams during LDAP sync or similar operations, it's sufficient to query only private rooms using methods like `findPrivateRoomsByIdsWithAbacAttributes`.
Applied to files:
apps/meteor/client/hooks/useRoomIcon.tsxapps/meteor/client/lib/rooms/roomTypes/private.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/client/hooks/useRoomIcon.tsxapps/meteor/client/lib/rooms/roomTypes/private.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/client/hooks/useRoomIcon.tsxapps/meteor/client/lib/rooms/roomTypes/private.ts
📚 Learning: 2025-10-30T19:30:46.541Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37244
File: apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.spec.tsx:125-146
Timestamp: 2025-10-30T19:30:46.541Z
Learning: In the AdminABACRoomAttributesForm component (apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.tsx), the first attribute value field is mandatory and does not have a Remove button. Only additional values beyond the first have Remove buttons. This means trashButtons[0] corresponds to the second value's Remove button, not the first value's.
Applied to files:
apps/meteor/client/hooks/useRoomIcon.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
apps/meteor/client/hooks/useRoomIcon.tsx (1)
11-17: ABAC icon precedence is correct; edge cases don't apply.Verification confirms ABAC rooms cannot be federated or discussion rooms. The codebase shows no instances of these combinations, and the learnings confirm ABAC attributes only exist on private rooms (type 'p'). The early return appropriately prioritizes the security indicator. The icon choices correctly differentiate teams ('team-shield') from channels ('hash-shield').
tassoevan
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
ABAC-71
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Refactor
Tests
Chores